feat: update auction in agoricNames, test that the boardId changed #9970

Chris-Hibbert · 2024-08-27T02:58:01Z

closes: #9963

Description

The upgrade of auctions neglected to replace the agoricNames binding for the auction instance. This fixes that and tests that the boardId is different before and after the coreEval.

Security Considerations

scary that we got that far through validation without noticing that the auction hadn't been updated in agoricNames.

Scaling Considerations

None.

Documentation Considerations

None.

dckc

looks good

a couple notes for future reference...

dckc · 2024-08-27T03:25:25Z

a3p-integration/proposals/a:vaults-auctions/upgradeVaults.test.js

+  t.true(
+    newAuctionInstance !== oldInstance,


Probably not worth a re-spin for this alone, but for future ref:

If you use t.not, ava will show the 2 values when the assertion fails.

Suggested change

t.true(

newAuctionInstance !== oldInstance,

t.not(newAuctionInstance,

oldInstance,

dckc · 2024-08-27T03:27:05Z

packages/inter-protocol/src/proposals/add-auction.js

+  auctionInstance.reset();
+  await auctionInstance.resolve(governedInstance);
+  // belt and suspenders; the above is supposed to also do this
+  await E(E(agoricNamesAdmin).lookupAdmin('instance')).update(


agoricNamesAdmin is a big hammer; it's unfortunate that resolve() alone didn't do the trick.

here's hoping for some later convenient time to figure out why not

the comment reads "belt-and-suspenders" as if the belt sometimes works. Does it not?

I haven't found the code that "is supposed to", so I don't know what it's trying to save, or what the corner cases might be. What code in promiseSpace is supposed to treat instances specially? Or what else updates agoricNames for instances?

Chris-Hibbert · 2024-08-27T04:05:03Z

The docker test failed wtih Error: key (an object) not found in collection "instanceToGovernor". That sounds like a flake to me, so I'll re-run it.

dckc · 2024-08-27T04:05:57Z

I don't think it's a flake. I think the econCommitteeCharter has to be introduced to the new auctioneer.

2024-08-27T03:55:06.783Z SwingSet: ls: v15: Error#1: key Object [Alleged: InstanceHandle] {} not found in collection instanceToGovernor
2024-08-27T03:55:06.783Z SwingSet: ls: v15: Error: key (an object) not found in collection "instanceToGovernor"
 at construct ()
 at Error (/bundled-source/.../node_modules/ses/src/error/tame-error-constructor.js:56)
 at makeError (/bundled-source/.../node_modules/ses/src/error/assert.js:243)
 at fail (/bundled-source/.../node_modules/ses/src/error/assert.js:357)
 at get (/bundled-source/.../packages/swingset-liveslots/src/collectionManager.js:351)
 at voteOnParamChanges (.../inter-protocol/src/econCommitteeCharter.js:79)

cloudflare-workers-and-pages · 2024-08-27T05:12:11Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`87b945d`
Status:	✅ Deploy successful!
Preview URL:	https://a03c723d.agoric-sdk.pages.dev
Branch Preview URL:	https://9963-registernewauction.agoric-sdk.pages.dev

View logs

Chris-Hibbert · 2024-08-27T05:15:29Z

Oops. It looks like you added your commit, and then I merge it again. I hope that was idempotent. I marked it for automerge:rebase.

turadg · 2024-08-27T14:08:03Z

a3p-integration/proposals/a:vaults-auctions/agd-tools.js

@@ -201,3 +201,18 @@ export const getProvisionPoolMetrics = async () => {
  const path = `published.provisionPool.metrics`;
  return getQuoteBody(path);
 };
+
+export const getAuctionInstance = async price => {


not really an "agd tool". Also would be more general with a little tweak:

Suggested change

export const getAuctionInstance = async price => {

export const getInstanceBoardId = async instanceName => {

(price isn't used)

Not a blocker for this repo but in synthetic-chain package it should be designed for re-use.

turadg · 2024-08-27T14:13:35Z

a3p-integration/proposals/a:vaults-auctions/eval.sh

+# we have an eval.sh so we can run prepare.sh before the rest
+
+echo "[$PROPOSAL] Running prepare.sh"
+./prepare.sh


prepare is for chain-halting upgrade proposals. There should be no prepare.sh in a CoreEvalProposal.

Please remove the indirection:

Suggested change

# we have an eval.sh so we can run prepare.sh before the rest

echo "[$PROPOSAL] Running prepare.sh"

./prepare.sh

./saveAuctionInstance.js

turadg · 2024-08-27T14:14:08Z

a3p-integration/proposals/a:vaults-auctions/prepare.sh

@@ -4,3 +4,5 @@
 # actions are executed in the upgraded chain software and the effects are
 # persisted in the generated image for the upgrade, so they can be used in
 # later steps, such as the "test" step, or further proposal layers.
+


another reason to remove this file is it says it's for after the proposal is executed but you're running it as part of the eval

turadg · 2024-08-27T14:15:19Z

a3p-integration/proposals/a:vaults-auctions/saveAuctionInstance.js

+console.log('old auction instance ', oldAuctionInstance, env.HOME);
+
+await writeFile(
+  `${env.HOME}/.agoric/previousInstance.json`,


this file is part of the permanent history a3p. please give it a more informative name for someone who comes across it without this context. or delete it after it's read.

also in general I think f a3p is writing to this directory it should be under a sub-path

turadg · 2024-08-27T14:16:55Z

packages/inter-protocol/src/proposals/add-auction.js

+  auctionInstance.reset();
+  await auctionInstance.resolve(governedInstance);
+  // belt and suspenders; the above is supposed to also do this
+  await E(E(agoricNamesAdmin).lookupAdmin('instance')).update(


the comment reads "belt-and-suspenders" as if the belt sometimes works. Does it not?

refs: #9970 ## Description Follow up on #9970 that had merged while I was reviewing ### Security Considerations none ### Scaling Considerations none ### Documentation Considerations none ### Testing Considerations CI ### Upgrade Considerations no additional

Chris-Hibbert added contract-upgrade auction next-release about next agoric-sdk or endo release labels Aug 27, 2024

Chris-Hibbert requested review from turadg and dckc August 27, 2024 02:58

Chris-Hibbert self-assigned this Aug 27, 2024

dckc approved these changes Aug 27, 2024

View reviewed changes

dckc added the force:integration Force integration tests to run on PR label Aug 27, 2024

Chris-Hibbert added the automerge:rebase Automatically rebase updates, then merge label Aug 27, 2024

Chris-Hibbert and others added 2 commits August 27, 2024 05:27

feat: update auction in agoricNames, test that the boardId changed

abf2168

chore: introduce economic committee charter to new auctioneer

87b945d

Chris-Hibbert force-pushed the 9963-registerNewAuction branch from ed7f1e6 to 87b945d Compare August 27, 2024 05:27

mergify bot merged commit 43345a5 into master Aug 27, 2024
83 checks passed

mergify bot deleted the 9963-registerNewAuction branch August 27, 2024 14:11

turadg requested changes Aug 27, 2024

View reviewed changes

turadg mentioned this pull request Aug 27, 2024

a3p vaults-auctions cleanup #9977

Merged

This was referenced Aug 27, 2024

chore: cleanup suggestions post-submission of 9970 #9979

Closed

test updating instances in promiseSpace updates agoricNames #9989

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update auction in agoricNames, test that the boardId changed #9970

feat: update auction in agoricNames, test that the boardId changed #9970

Chris-Hibbert commented Aug 27, 2024

dckc left a comment

dckc Aug 27, 2024

dckc Aug 27, 2024

turadg Aug 27, 2024

Chris-Hibbert Aug 27, 2024

Chris-Hibbert commented Aug 27, 2024

dckc commented Aug 27, 2024

cloudflare-workers-and-pages bot commented Aug 27, 2024 •

edited

Loading

Chris-Hibbert commented Aug 27, 2024

turadg Aug 27, 2024

turadg Aug 27, 2024

turadg Aug 27, 2024

turadg Aug 27, 2024

turadg Aug 27, 2024

	export const getAuctionInstance = async price => {
	export const getInstanceBoardId = async instanceName => {

feat: update auction in agoricNames, test that the boardId changed #9970

feat: update auction in agoricNames, test that the boardId changed #9970

Conversation

Chris-Hibbert commented Aug 27, 2024

Description

Security Considerations

Scaling Considerations

Documentation Considerations

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert commented Aug 27, 2024

dckc commented Aug 27, 2024

cloudflare-workers-and-pages bot commented Aug 27, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

Chris-Hibbert commented Aug 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Aug 27, 2024 •

edited

Loading